Switch custom Bzip2 cmake module to standard #2718
Conversation
|
Klaus Zimmermann seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
zklaus
force-pushed
the
cmake-bzip2-handling
branch
from
3669c38 to
a123e04
Compare
|
If memory serves, I found certain platforms had bz2 and not bzip2 and other platforms vice-versa. |
|
I have used this patch in the Conda-forge builds since conda-forge/libnetcdf-feedstock#178 and there it works on Windows, see here and here. Allow me to highlight that the approach currently in |
|
Its coming back to me again. Under Ubuntu-22, I found that apt |
|
Ah, that makes sense. The problem still needs to be addressed, though. Do you know where you got the Regardless, the good news is that the built-in module takes care of this by looking for both |
|
The only clue I see is this line: # Author: Siddhartha Chaudhuri, 2009 |
|
This change fails for me under Ubuntu 22. It says:
|
|
I'm seeing the same behavior as Dennis, but will investigate further. |
|
This is working for me on MacOS so I'll see if I can figure out what's happening on Ubuntu 22.04. |
|
what bzip2 package is being installed under MACOS? bz2 or bzip2? My guess us bzip2. |
|
Dennis, I'll have to dig to see what's installed as part of the development
packages re: 'bz2 vs. bzip2', but the library itself is libbz2, so I would
suspect bz2.
|
Yeah, I did see that and it is what lead me to https://github.com/sidch/CMake, which is a repository with a relatively large collection of Cmake modules by (presumably) the same author. I thought I might find the two modules used here there as well, but they are not present. The history of that repository stretches back only to August 2012 with an initial commit that already adds a lot of modules, so what I think happened is that the author has maintained the collection for some time, in 2009 including the Bzip2 modules, and removing them sometime between 2009 and the move of the collection to git in 2012, probably in favor of the upstream one.
Is that Ubuntu 22.04 Jammy or Ubuntu 22.10 Kinetic? With
Just to check I am not missing something: There aren't really two separate packages, right? It's all the same library, just that some packagers choose different names for their respective packages, right? As such, the question would maybe rather be "Which package manager are you using?" with Homebrew and Macports being the two most likely candidates for Macos. As binary on Linux, I have only seen |
| IF(Bz2_FOUND) | ||
| SET(TLL_LIBS ${TLL_LIBS} ${Bz2_LIBRARIES}) | ||
| IF(BZIP2_FOUND) | ||
| SET(TLL_LIBS ${TLL_LIBS} ${BZIP2_LIBRARIES}) |
There was a problem hiding this comment.
The FindBzip2 module defines an imported library, so might be better to use that, as it will take care of the header locations too:
| SET(TLL_LIBS ${TLL_LIBS} ${BZIP2_LIBRARIES}) | |
| SET(TLL_LIBS ${TLL_LIBS} BZip2::BZip2) |
(and below on the buildplugin line)
|
Re-checking these PR's in an attempt to get caught up on the backlog. |
Co-authored-by: Peter Hill <zed.three@gmail.com>
|
@K20shores Can you take a look at the conflict and confirm where this should go instead, keeping in line with the changes you've made? |
This replaces the custom Cmake modules
FindBz2.cmakeandFindBzip2.cmakewith the standardFindBZip2, included in Cmake. This entails a few changed variable names, but I tried to contain them in the Cmake parts of the build system, maintaining compatibility with the Autotools version.It is noteworthy that what appears to be the current incarnation of the original source of those custom modules at https://github.com/sidch/CMake/tree/main/Modules does not contain them anymore.
It is not completely clear to me why there are two different detection methods in the current sources, so I may be missing something, but it does lead to problems with the Bzip2 plugin installation (see #2717).
I also backported this change to 4.9.2, using it in the Conda-forge build, where it successfully installs the Bzip2 plugin.
Closes #2717.